fix(security)[P0]: enforce RLS on by-id writes — close member-edits-others'-records hole (#1985)#1994
Merged
Merged
Conversation
…s hole #1985 (P0). A single-id update/delete goes straight to driver.update(object, id)/driver.delete(object, id) and builds no query ast, so the RLS where filter the middleware injects on the read path was never applied to by-id writes. With member_default granting *: {edit,delete} (meant to be owner-scoped via the owner_only_writes/deletes RLS), the owner predicate was silently bypassed — any authenticated member could modify/delete another user's records (verified: a member PATCH'd an admin's record and it persisted). Two coordinated fixes: 1. Pre-image authorization check (security-plugin.ts): before a single-id update/delete, compute the write-op RLS filter and re-read the target row with { id } AND <writeFilter>; if not visible → PermissionDeniedError (403). Reuses existing RLS/tenant machinery, recursion-safe (find doesn't re-trigger it), skipped when no RLS policy applies (admins / modifyAllRecords unchanged). 2. Owner scoping keyed on a column that exists (default-permission-sets.ts): owner_only_writes/deletes keyed on owner_id, which author objects almost never declare → computeRlsFilter dropped the policy (the no-op that hid the bypass). Now keyed on created_by, the ownership column the engine stamps universally. Members may now write only the records they created; admins unrestricted. Audit-opt-out objects (no created_by) fail CLOSED for member writes. Verified live on app-crm (2 users): member→others' PATCH/DELETE = 403 (unmutated); member→own = 200; admin→any = 200. plugin-security 91 (+4), org-scoping 15, runtime 382, objectql 639 green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
📓 Docs Drift CheckThis PR changes 1 package(s): 8 hand-written doc(s) reference the affected code and may need an implementation-accuracy re-verification:
|
This was referenced Jun 17, 2026
Merged
os-zhuang
added a commit
that referenced
this pull request
Jun 18, 2026
…) (#2032) Adds the authorization dimension to the verifier — the machinery to catch the #1994 class (a by-id write that skips the row-level predicate, letting a member edit a record it cannot even see). - harness: `signUp(email)` mints a fresh member (the first user is the seeded admin, so a sign-up is a plain member with no grants — exactly what cross-owner proofs need). Also registers SharingServicePlugin so apps that declare `requires: ['sharing']` have their record-share grants enforced rather than silently inert (a harness-fidelity fix). - src/rls.ts: the app-agnostic invariant — "a user who cannot READ a record must not be able to WRITE it." Per object: admin creates a record; a fresh member tries to read it, then mutate it by id; re-read as admin decides if the row actually changed. Member-can't-read + row-changed = the #1994 hole. - test/rls-runner.test.ts: unit-proves the classifier flags a hole / passes when consistent / reports member-visible — so the detection logic is guaranteed. - test/auto-verify-rls.dogfood.test.ts: live smoke over CRM + showcase. HONEST CURRENT STATE: the harness boots single-tenant, so org-scoped RLS is stripped and a fresh member falls back to `member_default` (broad read). So both the example apps AND hotcrm report every object `member-visible` — the #1994 by-id-write path isn't exercised live yet. Making it a hard, revert-provable gate needs an owner-scoped fixture (private-default object + a member permission set carrying RLS.ownerPolicy + SecurityPlugin.fallbackPermissionSet). The runner is unit-proven and ready; the live invariant (zero holes) still guards against a regression. Fixture tracked as the next step. Private package — no changeset. Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
os-zhuang
added a commit
that referenced
this pull request
Jun 18, 2026
… gate The cross-owner RLS proof runner (src/rls.ts, #2032) was unit-proven but had nothing live to exercise it: the harness boots single-tenant, so the org tenant_isolation policy is stripped and a fresh member falls back to member_default (broad read) — every object reports `member-visible`, so the #1994 by-id-write invariant ("can't read ⇒ can't write") never fired live. Two faithful ways to create real isolation, both now live: 1. Owner-scoped fixture (test/rls-fixture.dogfood.test.ts + fixtures/): one object `rls_note` whose member fallback permission set carries RLS.ownerPolicy('rls_note','created_by'). Keyed on created_by/current_user.id (not organization_id), so it survives single-tenant stripping — a fresh member genuinely can't read the admin's note. - GREEN: owner policy on `all` ops → rls-consistent (pre-image check denies the by-id write). - AUTOMATED RED: owner policy on `select` only → rls-hole (read scoped, no write policy applies, by-id write lands) — #1994 hole class every CI run. - MANUAL REVERT PROOF (README): disabling the pre-image check flips the green fixture to "GET 404 yet MUTATED it by id (PATCH 200)". Verified, restored. 2. Multi-tenant org-scoped (test/rls-multitenant.dogfood.test.ts): bootDogfoodStack gains `{ multiTenant: true }` → registers plugin-org-scoping before SecurityPlugin so organization_id policies APPLY. CRM flips from every-object-member-visible (single-tenant) to 4 consistent / 0 holes / 0 member-visible — confirming member-visible was single-tenant RLS-stripping, not a broad-read default (the model real apps like hotcrm rely on). harness: BootOptions gains `security?` (SecurityPlugin override for the fixture fallback) and `multiTenant?`. README documents both paths + the revert proof. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
os-zhuang
added a commit
that referenced
this pull request
Jun 19, 2026
…h-risk classes (ADR-0054 #1 + Phase 2) (#2045) * feat(spec): add prove-it-runs proof field + ratchet to liveness gate (ADR-0054 #1) ADR-0054 follow-up (1): the liveness ledger's `live` meant only a static consumer pointer — necessary but not sufficient, since a property can be live at every layer yet broken end-to-end. This adds the third leg: high-risk authorable properties must carry a `proof` (a dogfood test reference) that asserts the runtime outcome. - proof-registry.mts: the authoritative high-risk-class list (field types, analytics, RLS, flow nodes, form widgets) + which classes the ratchet enforces this phase. Field types and RLS are bound (matrix exists AND surface governed); analytics/flow/form are listed-but-blocked with honest reasons (their surface isn't governed yet / no proof yet — Phase 2). - check-liveness.mts: a bound `live` entry must carry a valid `proof` of its own class. Validation is STATIC (file exists + declares the `@proof: <id>` tag) so the gate stays seconds-cheap; running the proof remains the dogfood gate's job. Reverse check flags unregistered `@proof:` tags. - Ledger: field.type → field-zoo proof; permission.rowLevelSecurity.using → #1994 RLS proof. Dogfood proofs self-declare their `@proof:` tag. - Gate now also triggers on packages/dogfood/** so deleting/renaming a proof re-runs the check and the dangling reference is caught. - 15 unit + wiring tests; README documents the contract and ratchet. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XVdnfUAx85amkerym26vdx * chore: add changeset for the liveness prove-it-runs gate change Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XVdnfUAx85amkerym26vdx * feat(verify,spec): bind flow-node high-risk class with a runtime proof (ADR-0054 Phase 2) Phase 2 extends the prove-it-runs ratchet to the flow-node class. - verify: bootStack gains an opt-in `automation` flag that registers @objectstack/service-automation, so authored flows are pulled from the registry and POST /automation/:name/trigger runs their nodes. Without it, flow execution was unreachable through the harness (the dispatcher's automation routes resolved no service). Mirrors the existing `multiTenant`/`security` opt-ins; default off. - dogfood: a self-contained flow fixture (one object + one autolaunched flow whose update_record node stamps a record) + the flow-node proof. It authors the flow, triggers it over HTTP, and asserts both directions — the targeted record is stamped (node executed) AND a bystander is untouched (the input variable wired into the node filter, not a blanket update). Runs green end-to-end. - spec: flow-node class is now `bound` in proof-registry.mts; flow.nodes.type carries the proof; the liveness gate enforces it. Three classes now bound: field types, RLS, flow nodes. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XVdnfUAx85amkerym26vdx * feat(spec): govern dataset + bind the analytics high-risk class (ADR-0054) Unlocks the analytics binding that was previously blocked because its authorable surface wasn't governed. - Govern the `dataset` metadata type: new liveness/dataset.json classifies all 19 authorable props with file:line evidence from the analytics service consumer audit (26 live, 1 dead — measures.certified has no runtime consumer). `dataset` added to GOVERNED in check-liveness.mts. - Bind the analytics class: dataset.dimensions.dateGranularity carries the tz-bucketing proof (#1982/#2018) — the property whose org-timezone day-bucket behavior the proof asserts. proof-registry.mts flips analytics to bound. - Four high-risk classes now CI-enforced: field types, RLS, flow nodes, analytics. - README governed-types + high-risk tables updated; changeset description updated. Verified: liveness gate green (dataset 27 classified); 17 registry tests; the analytics proof runs green; full dogfood suite 59/59. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XVdnfUAx85amkerym26vdx --------- Co-authored-by: Claude <noreply@anthropic.com>
os-zhuang
pushed a commit
that referenced
this pull request
Jun 19, 2026
…by-parent design Refocus from a high-level "gap-closure not rewrite" principle to the concrete landing plan for master-detail permission inheritance — the substance an ADR should carry. Grounded in an engine investigation: - Read: auto-derive the user's accessible master-id set and constrain the detail's master FK via the EXISTING pre-resolved-membership mechanism (ExecutionContext.rlsMembership + the compiler's `field IN (current_user.<key>)` form) — zero RLS-compiler changes. Trade-off table vs subquery-join (compiler has no subquery support) and materialized column (dual-write/stale). - Write: extend the #1994 by-id-write pre-image check to require master EDIT access. - Spec: add controlled_by_parent to the authorable sharingModel enum (fixes the OWDModel/sharingModel inconsistency); detail must declare one required master_detail field. - Proof (ADR-0054): a dogfood RLS proof; prerequisite is related-record topological synthesis in @objectstack/verify (P0). Honest limits: id-set size ceiling, single-level chains in v1. Status: Proposed. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XVdnfUAx85amkerym26vdx
os-zhuang
pushed a commit
that referenced
this pull request
Jun 19, 2026
…ADR-0055 P1+P2) Implements derived master-detail access — a detail object's read/write scope is derived from its master record. - spec: `controlled_by_parent` added to the authorable `object.sharingModel` enum (converges with OWDModel). - security (read): for a controlled_by_parent object, resolve the master records the user can read (reusing the master's own RLS via computeRlsFilter, run under a system context — no middleware re-entry) and AND-inject `masterFK IN (accessible master ids)` onto the read AST. Empty set ⇒ fail closed. Zero RLS-compiler changes (builds the $in filter directly). - security (write): extend the by-id-write path — a detail insert/update/delete requires CRUD edit on the master AND the master row visible under the master's write RLS. Closes the #1994-class by-id hole for derived access (the detail carries no authored RLS of its own). - proof (ADR-0054): dogfood RLS proof — a member who can't read the master can neither read nor by-id-write the detail, and isn't over-blocked on a master they own. Bound in the liveness ledger on object.sharingModel. Five high-risk classes now CI-enforced: field types, RLS, controlled-by-parent, analytics, flow nodes. Honest v1 limits documented in ADR-0055 (id-set ceiling, single-level chains). Full dogfood suite green (73 tests); liveness gate green; 18 registry tests. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XVdnfUAx85amkerym26vdx
os-zhuang
added a commit
that referenced
this pull request
Jun 19, 2026
…fy topological synthesis (ADR-0055) (#2049) * docs(adr): ADR-0055 — permission model evolves by gap-closure, not rewrite (Proposed) Records the architecture decision that the authorization model (already Salesforce-shaped: permission sets + FLS + predicate RLS + ownership + sharing rules + hierarchy) should close enumerable gaps via ADR-0049 enforce-or-remove and ADR-0054 prove-it-runs, against a mainstream conformance checklist — NOT a rewrite. Headline gap: master-detail `controlled_by_parent` is declared (OWDModel) yet has zero runtime consumers, isn't reachable through the object's `sharingModel` enum, and the RLS compiler is relationship-blind. Includes a conformance matrix and per-gap implement/remove/not-do verdicts. Status: Proposed. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XVdnfUAx85amkerym26vdx * docs(adr): rewrite ADR-0055 as the concrete master-detail controlled-by-parent design Refocus from a high-level "gap-closure not rewrite" principle to the concrete landing plan for master-detail permission inheritance — the substance an ADR should carry. Grounded in an engine investigation: - Read: auto-derive the user's accessible master-id set and constrain the detail's master FK via the EXISTING pre-resolved-membership mechanism (ExecutionContext.rlsMembership + the compiler's `field IN (current_user.<key>)` form) — zero RLS-compiler changes. Trade-off table vs subquery-join (compiler has no subquery support) and materialized column (dual-write/stale). - Write: extend the #1994 by-id-write pre-image check to require master EDIT access. - Spec: add controlled_by_parent to the authorable sharingModel enum (fixes the OWDModel/sharingModel inconsistency); detail must declare one required master_detail field. - Proof (ADR-0054): a dogfood RLS proof; prerequisite is related-record topological synthesis in @objectstack/verify (P0). Honest limits: id-set size ceiling, single-level chains in v1. Status: Proposed. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XVdnfUAx85amkerym26vdx * feat(verify): related-record topological synthesis (ADR-0055 P0) deriveCrudCases no longer skips objects with required relations. It builds the object dependency graph from required relational fields, topologically orders it (targets before dependents), and emits `relationalRefs` the runner fills with real ids — so relationship-dense objects (the core of real apps) are verified, not skipped. Both runners (runCrudVerification, runRlsProofs) thread a created-id registry via fillRelationalRefs. Honest `blocked` verdicts remain for the genuinely unsatisfiable: required relation to an external/missing target, required-reference cycle (incl. required self-reference), and cascade (target itself blocked). Optional relations never block — filled best-effort, else left null. This is the prerequisite harness for the master-detail controlled-by-parent RLS proof (ADR-0055 P2), and independently widens `objectstack verify` coverage. Tests: 10 unit cases (ordering, optional/required, external/missing, cascade, cycle, self-ref, id threading). Full dogfood suite green (69 tests); rls cross-owner counts unchanged on example-crm. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XVdnfUAx85amkerym26vdx * feat(security,spec): master-detail controlled-by-parent permissions (ADR-0055 P1+P2) Implements derived master-detail access — a detail object's read/write scope is derived from its master record. - spec: `controlled_by_parent` added to the authorable `object.sharingModel` enum (converges with OWDModel). - security (read): for a controlled_by_parent object, resolve the master records the user can read (reusing the master's own RLS via computeRlsFilter, run under a system context — no middleware re-entry) and AND-inject `masterFK IN (accessible master ids)` onto the read AST. Empty set ⇒ fail closed. Zero RLS-compiler changes (builds the $in filter directly). - security (write): extend the by-id-write path — a detail insert/update/delete requires CRUD edit on the master AND the master row visible under the master's write RLS. Closes the #1994-class by-id hole for derived access (the detail carries no authored RLS of its own). - proof (ADR-0054): dogfood RLS proof — a member who can't read the master can neither read nor by-id-write the detail, and isn't over-blocked on a master they own. Bound in the liveness ledger on object.sharingModel. Five high-risk classes now CI-enforced: field types, RLS, controlled-by-parent, analytics, flow nodes. Honest v1 limits documented in ADR-0055 (id-set ceiling, single-level chains). Full dogfood suite green (73 tests); liveness gate green; 18 registry tests. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XVdnfUAx85amkerym26vdx * chore: ADR-0055 Accepted + changeset; fix pre-existing app-showcase typecheck - ADR-0055 status → Accepted (implemented in this PR, P0–P2). - changeset for the controlled-by-parent feature (spec/plugin-security/verify minor). - fix(app-showcase): task-triage userFilters tabs were missing the now-required pinned/isDefault/visible fields (a pre-existing main breakage from #2048 that reddened TypeScript Type Check on every PR) — examples typecheck now green. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XVdnfUAx85amkerym26vdx * feat(showcase): controlled-by-parent invoice lines + HTTP verification (ADR-0055) Adds the canonical master-detail scenario to the showcase: `showcase_invoice_line` now declares `sharingModel: 'controlled_by_parent'`, so a line's access is derived from its parent `showcase_invoice` (a line is meaningless apart from its invoice). Admin/default single-tenant behavior is unchanged (admin sees all invoices → all lines); the showcase's own 20 tests still pass. Verification (real HTTP stack — the boundary is server-side; no browser needed): a dogfood proof boots the REAL showcase app with a member permission set carrying an owner policy on the invoice, then asserts a member who cannot read an admin-owned invoice can neither read nor by-id-write its lines, but can read/edit lines under an invoice they own. 4/4 green; full dogfood suite 77 tests. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XVdnfUAx85amkerym26vdx --------- Co-authored-by: Claude <noreply@anthropic.com>
This was referenced Jun 19, 2026
os-zhuang
pushed a commit
that referenced
this pull request
Jul 4, 2026
… destructive-op gates Correctness (record-level enforcement for the pre-mapped destructive ops): - security-plugin middleware: extend the write-scope stash, the #1994 pre-image RLS check, and the ADR-0055 controlled-by-parent write gate to cover transfer/restore/purge — the object-level RBAC bit was gated but the row-level defenses only listed update/delete/insert, so a granted user could have destroyed out-of-scope rows by id when the M2 ops ship. purge maps onto the delete RLS class, transfer/restore onto update. New unit test proves a not-owned purge is denied by the pre-image check. Callsite coverage for the default flip: - cli serve: carve-out now keys on (tierEnabled('auth') || hasAuthPlugin) — a stack mounting AuthPlugin explicitly under a minimal tier no longer gets an accidental fail-open override. - plugin-dev: mirror the serve carve-out — an auth-less dev stack gets an explicit requireAuth:false so the local data API isn't bricked. - spec: declare ObjectStackDefinitionSchema.api so the documented opt-out (and the enableProjectScoping/projectResolution/enforceProjectMembership knobs serve.ts already reads) survives defineStack strict parsing instead of being silently stripped. Verified: defineStack now returns the api key. - rest-api-plugin: warn only on an explicit opt-out; add a misplaced-key guard for a flat api.requireAuth (silently ignored under the deny default). - fix two in-tree tests that passed requireAuth at the wrong nesting. Cleanup: - derive the modifyAllRecords write-bypass key set from OPERATION_TO_PERMISSION + DESTRUCTIVE_OPERATIONS (module-level Set, no per-call array) so a future destructive op is covered automatically. Docs / provenance: - regenerate the auto-gen references (requireAuth + allow*{Transfer,Restore, Purge} describe text); CHANGELOG entry for the breaking flip; changeset migration note updated to the defineStack-level api opt-out + scope note. Pre-existing anonymous-posture gap on /meta, dispatcher /graphql, and raw hono /data (surfaces that never call enforceAuth) filed as #2567 — out of scope for this flip. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014y5kiH3aPLWtRRRGcVrXcT
os-zhuang
added a commit
that referenced
this pull request
Jul 4, 2026
…transfer/restore/purge (#2561 P0) (#2562) * feat(security)!: secure-by-default requireAuth flip + RBAC gates for transfer/restore/purge (#2561 P0) Two production-launch blockers from the authorization gap map (#2561): 1. requireAuth default flip (ADR-0056 D2, enforce stage) — BREAKING - spec: RestApiConfigSchema.requireAuth default(false) → default(true); rest-server normalizeConfig mirrors (?? true). Anonymous /data/* is now denied (401) unless the deployment explicitly opts out. - rest-api-plugin: boot warning now fires only for the EXPLICIT requireAuth:false opt-out, and reads the correct nested config path (the flat read previously warned even when requireAuth was on). - cli serve: keeps the deliberate carve-out — auth-less stacks get an explicit false (nothing could authenticate against them), warned. - verify harness: no override — every dogfood proof now runs on the platform default; anonymous-deny + public-form survival prove the flip (publicFormGrant was the pre-req that unblocked it). - conformance matrix: requireAuth-default-flip experimental → enforced. - REST/client unit tests that dispatch handlers anonymously opt out explicitly (they test routing mechanics, not the auth gate). 2. #1883 — destructive lifecycle ops RBAC gate (pre-mapped) - permission-evaluator: OPERATION_TO_PERMISSION now maps transfer→allowTransfer, restore→allowRestore, purge→allowPurge; modifyAllRecords super-user bypass covers the class. Ops still do not exist in ObjectQL (M2) — but there is no ungated window when they ship; unmapped destructive ops keep failing closed (ADR-0049). - spec: bits re-described [EXPERIMENTAL — not enforced] → [RBAC-gated; operation pending M2]; liveness ledger entries added. - unit tests: deny-without-bit / allow-with-bit / no-cross-leak / modifyAllRecords bypass. Verified: full turbo test suite green (129 tasks); browser-driven showcase run proves anonymous 401, authenticated 200, anonymous public form 200/201 with the data API still closed to the same session. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014y5kiH3aPLWtRRRGcVrXcT * docs: honesty caveats for pending transfer/restore/purge ops in permission docs The permissions-matrix cheatsheet and ObjectQL security protocol page listed allowTransfer/allowRestore/allowPurge without noting the operations do not exist yet (roadmap M2) — per the declared≠enforced honesty rule, both now carry the pending-M2 / gate-pre-mapped caveat (#1883). Also documents the modifyAllRecords bypass extension to the lifecycle class. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014y5kiH3aPLWtRRRGcVrXcT * docs(adr): record FLS posture limitation + capability authoring-lint in ADR-0066 future refinements Two findings from the 2026-07 pre-launch authorization assessment that had no ADR home: - ⑧ runtime FLS is block-list + most-permissive union — undeclared fields are visible by default and a field-level deny cannot be expressed until the muting layer (⑦) covers field grants; - ⑨ capability strings need an authoring/publish-gate lint when the D1 registry lands (typos currently fail closed but undiscoverably). Companion engineering issue (not ADR material): #2565 — surface swallowed dbLoader failures in resolvePermissionSets. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014y5kiH3aPLWtRRRGcVrXcT * fix(security): address self-review findings on the requireAuth flip + destructive-op gates Correctness (record-level enforcement for the pre-mapped destructive ops): - security-plugin middleware: extend the write-scope stash, the #1994 pre-image RLS check, and the ADR-0055 controlled-by-parent write gate to cover transfer/restore/purge — the object-level RBAC bit was gated but the row-level defenses only listed update/delete/insert, so a granted user could have destroyed out-of-scope rows by id when the M2 ops ship. purge maps onto the delete RLS class, transfer/restore onto update. New unit test proves a not-owned purge is denied by the pre-image check. Callsite coverage for the default flip: - cli serve: carve-out now keys on (tierEnabled('auth') || hasAuthPlugin) — a stack mounting AuthPlugin explicitly under a minimal tier no longer gets an accidental fail-open override. - plugin-dev: mirror the serve carve-out — an auth-less dev stack gets an explicit requireAuth:false so the local data API isn't bricked. - spec: declare ObjectStackDefinitionSchema.api so the documented opt-out (and the enableProjectScoping/projectResolution/enforceProjectMembership knobs serve.ts already reads) survives defineStack strict parsing instead of being silently stripped. Verified: defineStack now returns the api key. - rest-api-plugin: warn only on an explicit opt-out; add a misplaced-key guard for a flat api.requireAuth (silently ignored under the deny default). - fix two in-tree tests that passed requireAuth at the wrong nesting. Cleanup: - derive the modifyAllRecords write-bypass key set from OPERATION_TO_PERMISSION + DESTRUCTIVE_OPERATIONS (module-level Set, no per-call array) so a future destructive op is covered automatically. Docs / provenance: - regenerate the auto-gen references (requireAuth + allow*{Transfer,Restore, Purge} describe text); CHANGELOG entry for the breaking flip; changeset migration note updated to the defineStack-level api opt-out + scope note. Pre-existing anonymous-posture gap on /meta, dispatcher /graphql, and raw hono /data (surfaces that never call enforceAuth) filed as #2567 — out of scope for this flip. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014y5kiH3aPLWtRRRGcVrXcT --------- Co-authored-by: Claude <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes the P0 broken-access-control vulnerability found driving app-crm end-to-end (#1985): any authenticated member could read and modify/delete any other user's records.
Root cause
A single-id
update/deletegoes straight todriver.update(object, id, …)/driver.delete(object, id)and builds no queryast— so the RLSwherefilter the security middleware injects on the read path (if (opCtx.ast) …) is never applied to by-id writes. Meanwhilemember_defaultgrants*: { edit, delete }, intended to be owner-scoped by theowner_only_writes/deletesRLS. Two failures compounded:owner_id, which author-defined objects almost never declare; the engine stampscreated_by, notowner_id.computeRlsFilterdropped the missing-column policy, so the predicate silently no-op'd.Verified: a member
PATCH'd an admin's record and the change persisted (admin re-read it).Fix (two coordinated changes)
1. Pre-image authorization check (
security-plugin.ts). Before a single-idupdate/delete, compute the write-operation RLS filter and re-read the target row with{ id } AND <writeFilter>; if it isn't visible (someone else's row, or RLS-hidden) →PermissionDeniedError(403).finddoesn't re-enter the check.modifyAllRecords, unguarded objects behave exactly as before.2. Owner scoping on a column that exists (
default-permission-sets.ts).owner_only_writes/owner_only_deletesnow key oncreated_by— the ownership column the registry injects on every object (registry.ts:263) — instead of the usually-absentowner_id.Behavior after fix
modifyAllRecords→ anysystemFields.audit: false, nocreated_by) now fail closed for member writes — grantmodifyAllRecordsor a per-object policy to allow. (Safe direction.)Verification
plugin-security91,plugin-org-scoping15,runtime382,objectql639 green.Scope / follow-up
This closes the owner (intra-tenant) write-authorization hole — the verified vuln. Cross-tenant isolation additionally needs an organization assigned at sign-up (fresh users currently have
activeOrg = None, sotenant_isolationdoesn't bind); tracked in #1985. The pre-image check here will enforce the tenant predicate on writes too once org-scoping is active.🤖 Generated with Claude Code